[vslib] Fix VS SAI reporting 0xFFFFFFFF oper speed for virtio NICs by rustiqly · Pull Request #1763 · sonic-net/sonic-sairedis

rustiqly · 2026-02-11T00:40:21Z

What I did

[agent]
When running SONiC VS on KVM/virtio, /sys/class/net/ethN/speed returns -1 (unknown speed). vs_get_oper_speed() reads this into a uint32_t, which wraps to 4294967295 (0xFFFFFFFF) and gets reported as SAI_PORT_ATTR_OPER_SPEED. This causes show interfaces status to display 4294967.3G as the port speed for operationally up ports.

How I did it

vs_get_oper_speed(): Read sysfs speed as int32_t instead of directly into uint32_t. Check for <= 0 (invalid) and return false with a warning log.
refresh_port_oper_speed(): When vs_get_oper_speed() fails, fall back to SAI_PORT_ATTR_SPEED (configured speed from CONFIG_DB) instead of returning SAI_STATUS_FAILURE.

How to verify it

Build and run SONiC VS on KVM with virtio NICs
Before fix: show interfaces status shows 4294967.3G for Ethernet0/4/8
After fix: Shows correct configured speed (e.g. 40G)

Or verify directly:

# virtio NIC reports -1 for speed
cat /sys/class/net/eth1/speed
# -1

# STATE_DB now shows configured speed instead of 0xFFFFFFFF
redis-cli -n 6 HGET 'PORT_TABLE|Ethernet0' speed
# 40000

Previous command output (if the output of a command-Loss, currentError etc has changed)

Before: 4294967.3G
After: 40G

Signed-off-by: Rustiqly rustiqly@users.noreply.github.com

mssonicbld · 2026-02-11T00:40:29Z

/azp run

azure-pipelines · 2026-02-11T00:40:40Z

Azure Pipelines successfully started running 1 pipeline(s).

rustiqly · 2026-02-11T00:41:43Z

Companion PR: sonic-net/sonic-buildimage#25428 (enables SAI_VS_USE_CONFIGURED_SPEED_AS_OPER_SPEED=true in all VS platform sai.profile files)

lguohan · 2026-02-11T00:47:28Z

@rustiqly , can you also add unit test for this PR?

rustiqly · 2026-02-11T01:48:34Z

Added 3 unit tests in unittest/vslib/TestSwitchBCM56850.cpp:

test_refresh_port_oper_speed_configured_speed — verifies that when m_useConfiguredSpeedAsOperSpeed=true, oper speed equals the configured speed (40G)
test_refresh_port_oper_speed_down_port — verifies oper speed is 0 for operationally down ports
test_refresh_port_oper_speed_fallback_no_tap — verifies that when vs_get_oper_speed() fails (no TAP/hostif), refresh_port_oper_speed() falls back to configured speed instead of returning SAI_STATUS_FAILURE

Also building a VS image with both fixes to verify end-to-end on KVM.

mssonicbld · 2026-02-11T01:49:23Z

/azp run

azure-pipelines · 2026-02-11T01:49:33Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-02-11T08:08:30Z

/azp run

rustiqly · 2026-02-11T08:08:36Z

@lguohan Fixed — the CI failure was aspellcheck.pl (spellcheck), not a unit test logic failure. Added 'NIC', 'NICs', 'oper', 'sysfs', 'virtio' to tests/aspell.en.pws. Rebased and force-pushed.

azure-pipelines · 2026-02-11T08:08:41Z

Azure Pipelines successfully started running 1 pipeline(s).

Copilot

Pull request overview

Fixes incorrect reporting of SAI_PORT_ATTR_OPER_SPEED in VS when sysfs returns “unknown” speed (e.g., -1 on virtio), avoiding the 0xFFFFFFFF wraparound and using configured port speed as a fallback.

Changes:

Update vs_get_oper_speed() to read sysfs speed as signed and reject invalid values (<= 0) with a warning.
Update refresh_port_oper_speed() to fall back to SAI_PORT_ATTR_SPEED when operational speed can’t be read.
Add unit tests intended to cover configured-speed and fallback behavior; update spellcheck word list.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
vslib/SwitchStateBaseHostif.cpp	Reads sysfs speed as `int32_t` and rejects invalid/unknown values before assigning to `uint32_t`.
vslib/SwitchStateBase.cpp	Falls back to configured port speed when operational speed read fails.
unittest/vslib/TestSwitchBCM56850.cpp	Adds tests for oper-speed refresh scenarios (but currently calls a protected method directly).
tests/aspell.en.pws	Adds new words used by comments/logs/tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

unittest/vslib/TestSwitchBCM56850.cpp

vslib/SwitchStateBaseHostif.cpp

unittest/vslib/TestSwitchBCM56850.cpp

rustiqly · 2026-02-11T08:53:45Z

@lguohan Found it — the failing test was SwitchBCM81724.refresh_read_only (line 150), which expected get(OPER_SPEED) to fail when no TAP device exists. My fallback-to-configured-speed change in refresh_port_oper_speed() now makes it succeed instead. Updated the test to match the new behavior. Force-pushed.

mssonicbld · 2026-02-11T08:57:27Z

/azp run

azure-pipelines · 2026-02-11T08:57:39Z

Azure Pipelines successfully started running 1 pipeline(s).

rustiqly · 2026-02-11T16:40:04Z

/azp run

azure-pipelines · 2026-02-11T16:40:12Z

Commenter does not have sufficient privileges for PR 1763 in repo sonic-net/sonic-sairedis

rustiqly · 2026-02-11T16:40:13Z

@lguohan The CI failure is FlexCounter.bulkChunksize (TestFlexCounter.cpp:1390) — an unrelated flaky test. Our changes only touch vslib/SwitchStateBase*.cpp and unittest/vslib/TestSwitchBCM*.cpp.

All our tests passed:

SwitchBCM56850.test_refresh_port_oper_speed_configured_speed  OK (12 ms)
SwitchBCM56850.test_refresh_port_oper_speed_down_port         OK (12 ms)
SwitchBCM56850.test_refresh_port_oper_speed_fallback_no_tap   OK (11 ms)
SwitchBCM81724.refresh_read_only                              OK (5 ms)

Triggered a rerun.

mssonicbld · 2026-02-11T17:37:48Z

/azp run

azure-pipelines · 2026-02-11T17:38:00Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-02-12T08:17:15Z

/azp run

azure-pipelines · 2026-02-12T08:17:27Z

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines · 2026-03-13T14:03:28Z

Azure Pipelines successfully started running 1 pipeline(s).

rustiqly · 2026-03-13T20:25:48Z

CI failures are redis socket errors in vstest harness ("Unable to connect to redis (unix-socket)") — known infra flakiness unrelated to this change. All p4rt tests passed. Re-triggering.

/azp run

mssonicbld · 2026-03-15T14:06:16Z

/azp run

rustiqly · 2026-03-15T14:06:24Z

@lguohan Rebased to latest master (d189ce6). The speed = 0 assignment before return false has been in place since the test addition. Also updated the commit message per @yejianquan's review — it now accurately describes the behavior:

Default: reports 0 (unknown) when sysfs speed is unavailable
With m_useConfiguredSpeedAsOperSpeed=true: uses configured speed (bypasses sysfs)

Could you take another look? Thanks!

azure-pipelines · 2026-03-15T14:06:28Z

Azure Pipelines successfully started running 1 pipeline(s).

rustiqly · 2026-03-15T15:47:28Z

VS Image Build + Test Results

Built a VS image with this PR commit (d189ce6) on latest master and deployed to KVM:

Build: SONiC.master.0-3f21a277b (sonic-vs.img.gz, 1.9GB, clean build, 0 errors)

Bug trigger confirmed: sysfs reports -1 for virtio NIC speed:

eth1/speed: -1
eth2/speed: -1
Without fix: uint32_t(-1) = 4294967295 (0xFFFFFFFF)

sai.profile active (companion PR #25428 merged):

SAI_VS_USE_CONFIGURED_SPEED_AS_OPER_SPEED=true

show interfaces status — correct 40G speed, no garbage values:

  Interface            Lanes    Speed    MTU    FEC           Alias    Vlan    Oper    Admin
-----------  ---------------  -------  -----  -----  --------------  ------  ------  -------
  Ethernet0      25,26,27,28      40G   9100    N/A    fortyGigE0/0  routed      up       up
  Ethernet4      29,30,31,32      40G   9100    N/A    fortyGigE0/4  routed      up       up
  Ethernet8      33,34,35,36      40G   9100    N/A    fortyGigE0/8  routed      up       up
 Ethernet12      37,38,39,40      40G   9100    N/A   fortyGigE0/12  routed    down       up

No error logs: 0 matches for 0xFFFFFFFF, 4294967295, or invalid.*speed in syncd logs.

3 ports oper=up, all showing correct 40G configured speed.

mssonicbld · 2026-03-25T18:26:08Z

/azp run

azure-pipelines · 2026-03-25T18:26:20Z

Azure Pipelines successfully started running 1 pipeline(s).

When running on KVM/virtio, /sys/class/net/ethN/speed returns -1 (unknown). vs_get_oper_speed() reads this into uint32_t, which wraps to 4294967295 (0xFFFFFFFF) and gets reported as the oper speed. Fix: - Read sysfs speed as int32_t and check for <= 0 (invalid) - When invalid, log a warning, set speed=0, and return false - In refresh_port_oper_speed(), set attr.value.u32=0 (unknown) instead of returning SAI_STATUS_FAILURE - When m_useConfiguredSpeedAsOperSpeed is true, configured speed is used as oper speed (bypassing sysfs entirely) This ensures VS ports show 0 (unknown) rather than garbage values on virtual NICs. With the companion sai.profile change (#25428), VS ports will show the correct configured speed. Added unit tests: - test_refresh_port_oper_speed_configured_speed: verifies oper speed equals configured speed when m_useConfiguredSpeedAsOperSpeed=true - test_refresh_port_oper_speed_down_port: verifies oper speed is 0 for operationally down ports - test_refresh_port_oper_speed_fallback_no_tap: verifies fallback when vs_get_oper_speed fails (no TAP device) Signed-off-by: Rustiqly <rustiqly@users.noreply.github.com>

mssonicbld · 2026-03-26T14:01:09Z

/azp run

azure-pipelines · 2026-03-26T14:01:22Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-03-26T23:41:51Z

/azp run

rustiqly · 2026-03-26T23:42:05Z

Fix for BuildAsan FlexCounter.bulkChunksize failure

Pushed an incremental commit (4571f8f) to fix the persistent FlexCounter.bulkChunksize test failure under AddressSanitizer.

Root Cause

Under Asan, the test thread runs 2-5x slower while the FlexCounter worker thread polls at its normal 1000ms interval. This creates a race:

Test creates FlexCounter with status=enable and poll_interval=1000
Test adds 6 port counters one by one
FlexCounter worker polls — but under Asan, only 1 port may be registered
Test sets BULK_CHUNK_SIZE=3 — but the worker already consumed initialCheckCount

Result: mock_bulkGetStats gets object_count=1 when expecting 3, causing assertion failure.

Fix

Added an std::atomic<bool> chunkConfigReady flag that synchronizes the test thread and worker thread:

In mock_bulkGetStats: after the initialCheckCount guard, if chunkConfigReady is false, treat the call as an extra initialization poll — populate counters but skip chunk-size assertions
In testAddRemoveCounter: added an optional onChunkConfigApplied callback that fires after chunk config is applied to FlexCounter
Per test case: flag is reset to false before calls where bulkChunkSizeAfterPort=true (config after ports → race possible), and pre-set to true for bulkChunkSizeAfterPort=false or forceSingleCall=true cases (no race)

This eliminates the timing dependency entirely — no sleeps, no increased timeouts, just explicit synchronization.

mssonicbld · 2026-03-26T23:53:36Z

/azp run

azure-pipelines · 2026-03-26T23:53:42Z

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

mssonicbld · 2026-03-26T23:53:47Z

/azp run

azure-pipelines · 2026-03-26T23:53:58Z

Azure Pipelines successfully started running 1 pipeline(s).

lihuay · 2026-03-29T15:33:29Z

/azp run

azure-pipelines · 2026-03-29T15:33:39Z

Azure Pipelines successfully started running 1 pipeline(s).

…oll-wait ## Description Fix flaky `FlexCounter.bulkChunksize` unit test that intermittently fails in CI due to timing-dependent `usleep(1000*1050)`. **Issue:** sonic-net#1765 ## Root Cause The test uses `usleep(1000*1050)` (1.05s) to wait for the FlexCounter polling thread (1s poll interval) to complete counter collection. Under CI load, the polling thread may not finish within this window, causing assertion failures like: ``` TestFlexCounter.cpp:1390: Failure Expected equality of these values: object_count Which is: 6 unifiedBulkChunkSize Which is: 3 ``` This failure reproduces on completely unrelated PRs ([sonic-net#1763](sonic-net#1763), [sonic-net#1764](sonic-net#1764)), confirming it is timing-dependent and not caused by code changes. ## Fix Replace all `usleep(1000*1050)` calls with deterministic poll-wait helpers: - **`waitForCounterKeys(table, expectedCount)`** — polls until the expected number of counter keys appear in COUNTERS_DB - **`waitForCounterValue(table, key, field)`** — polls until a specific counter field has a non-empty value Both helpers poll every 100ms with a 5-second timeout. This eliminates timing sensitivity while keeping tests fast on unloaded machines (typically completes in 1-2 polls). Also replaces the similar `usleep(1000*1000)` and `usleep(1000*2000)` in `queryCounterCapability` / `addRemoveCounterForPort` which have the same timing vulnerability. ## Changes - `unittest/syncd/TestFlexCounter.cpp`: - Add `#include <chrono>` for `steady_clock` - Add `waitForCounterKeys()` helper - Add `waitForCounterValue()` helper - Replace 8x `usleep(1000*1050)`, 1x `usleep(1000*1000)`, 1x `usleep(1000*2000)` across `testAddRemoveCounter`, `FlexCounter.counterIdChange`, `FlexCounter.addRemoveCounterForPort`, and `testDashMeterAddRemoveCounter` - Retain `usleep(60*1000)` in `updateSwitchDebugCounterIdList` (50ms poll interval, not flaky) Signed-off-by: Sonic Build Admin <sonicbld@microsoft.com>

rustiqly mentioned this pull request Feb 11, 2026

[vs] Enable SAI_VS_USE_CONFIGURED_SPEED_AS_OPER_SPEED in VS platform sai.profile sonic-net/sonic-buildimage#25428

Merged

rustiqly force-pushed the fix/vs-oper-speed-negative branch from 70620e1 to 1580a71 Compare February 11, 2026 01:48

rustiqly force-pushed the fix/vs-oper-speed-negative branch from 1580a71 to 400fff5 Compare February 11, 2026 08:08

lguohan requested a review from Copilot February 11, 2026 08:20

Copilot started reviewing on behalf of lguohan February 11, 2026 08:21 View session

Copilot AI reviewed Feb 11, 2026

View reviewed changes

rustiqly force-pushed the fix/vs-oper-speed-negative branch from 400fff5 to 9ce11d3 Compare February 11, 2026 08:53

This was referenced Feb 11, 2026

[test] FlexCounter.bulkChunksize is flaky due to timing-dependent usleep #1765

Open

[test] Fix flaky FlexCounter.bulkChunksize by replacing usleep with poll-wait #1766

Merged

rustiqly force-pushed the fix/vs-oper-speed-negative branch from 9ce11d3 to 1b69b8e Compare February 11, 2026 17:37

rustiqly mentioned this pull request Mar 14, 2026

[VS] All subport tests are consistently failing sonic-net/sonic-swss#2365

Closed

rustiqly force-pushed the fix/vs-oper-speed-negative branch from 699f972 to d189ce6 Compare March 15, 2026 14:06

securely1g previously approved these changes Mar 16, 2026

View reviewed changes

lguohan previously approved these changes Mar 16, 2026

View reviewed changes

rustiqly dismissed stale reviews from lguohan and securely1g via 094830d March 25, 2026 18:26

rustiqly force-pushed the fix/vs-oper-speed-negative branch from d189ce6 to 094830d Compare March 25, 2026 18:26

rustiqly force-pushed the fix/vs-oper-speed-negative branch from 094830d to 7c239ff Compare March 26, 2026 14:01

rustiqly force-pushed the fix/vs-oper-speed-negative branch from 1e3b4ea to 7c239ff Compare March 26, 2026 23:53

mssonicbld mentioned this pull request Mar 31, 2026

[action] [PR:1766] [test] Fix flaky FlexCounter.bulkChunksize by replacing usleep with poll-wait #1823

Open

Conversation

rustiqly commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What I did

How I did it

How to verify it

Previous command output (if the output of a command-Loss, currentError etc has changed)

Uh oh!

mssonicbld commented Feb 11, 2026

Uh oh!

azure-pipelines bot commented Feb 11, 2026

Uh oh!

rustiqly commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lguohan commented Feb 11, 2026

Uh oh!

rustiqly commented Feb 11, 2026

Uh oh!

mssonicbld commented Feb 11, 2026

Uh oh!

azure-pipelines bot commented Feb 11, 2026

Uh oh!

mssonicbld commented Feb 11, 2026

Uh oh!

rustiqly commented Feb 11, 2026

Uh oh!

azure-pipelines bot commented Feb 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rustiqly commented Feb 11, 2026

Uh oh!

mssonicbld commented Feb 11, 2026

Uh oh!

azure-pipelines bot commented Feb 11, 2026

Uh oh!

rustiqly commented Feb 11, 2026

Uh oh!

azure-pipelines bot commented Feb 11, 2026

Uh oh!

rustiqly commented Feb 11, 2026

Uh oh!

mssonicbld commented Feb 11, 2026

Uh oh!

azure-pipelines bot commented Feb 11, 2026

Uh oh!

mssonicbld commented Feb 12, 2026

Uh oh!

azure-pipelines bot commented Feb 12, 2026

Uh oh!

azure-pipelines bot commented Mar 13, 2026

Uh oh!

rustiqly commented Mar 13, 2026

Uh oh!

mssonicbld commented Mar 15, 2026

Uh oh!

rustiqly commented Mar 15, 2026

Uh oh!

azure-pipelines bot commented Mar 15, 2026

Uh oh!

rustiqly commented Mar 15, 2026

VS Image Build + Test Results

Uh oh!

mssonicbld commented Mar 25, 2026

Uh oh!

azure-pipelines bot commented Mar 25, 2026

Uh oh!

mssonicbld commented Mar 26, 2026

Uh oh!

azure-pipelines bot commented Mar 26, 2026

Uh oh!

rustiqly commented Feb 11, 2026 •

edited

Loading

rustiqly commented Feb 11, 2026 •

edited

Loading